refactor: replace sort.Slice with slices.Sort for natural ordering by ledigang · Pull Request #3255 · dydxprotocol/v4-chain

ledigang · 2025-11-21T10:25:01Z

Changelist

There is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

Test Plan

[Describe how this PR was tested (if applicable)]

Author/Reviewer Checklist

If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

Refactor
- Updated internal sorting utilities across multiple modules to use modern library functions, improving code maintainability. Existing functionality remains unchanged.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Signed-off-by: ledigang <shuangcui@msn.com>

coderabbitai · 2025-11-21T10:25:36Z

Walkthrough

The PR updates four Go source files to replace sort.Slice function calls with slices.Sort from the standard library's slices package, standardizing the sorting approach while preserving identical sorting behavior.

Changes

Cohort / File(s)	Summary
Sort.Slice to slices.Sort migration `protocol/lib/math.go`, `protocol/x/clob/keeper/grpc_query_leverage.go`, `protocol/x/perpetuals/keeper/perpetual.go`, `protocol/x/prices/client/testutil/util.go`	Replace `sort.Slice` calls with `slices.Sort` and update imports from `sort` to `slices` package. Maintains identical sorting behavior and deterministic ordering across all usages.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Homogeneous refactoring pattern applied consistently across all four files
No logic changes, no behavioral modifications
Simple import swaps and direct function call replacements
No exported API changes

Poem

🐰 Whiskers twitching with delight,
Sort to slices—what a sight!
Four files neat, now unified,
Modern Go with style, applied! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main refactoring change: replacing sort.Slice with slices.Sort across multiple files for better code clarity.
Description check	✅ Passed	The description includes the required Changelist and Author/Reviewer Checklist sections. However, the Test Plan section is left incomplete with only the template placeholder text.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2121b2 and 4e8c407.

📒 Files selected for processing (4)

protocol/lib/math.go (2 hunks)
protocol/x/clob/keeper/grpc_query_leverage.go (2 hunks)
protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
protocol/x/prices/client/testutil/util.go (2 hunks)

🧰 Additional context used

🧠 Learnings (5)

📚 Learning: 2024-11-15T16:17:29.092Z

Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2551
File: protocol/x/clob/types/expected_keepers.go:86-90
Timestamp: 2024-11-15T16:17:29.092Z
Learning: The function `GetCrossInsuranceFundBalance` in `protocol/x/clob/types/expected_keepers.go` already existed and was just moved in this PR; changes to its error handling may be out of scope.

Applied to files:

protocol/x/perpetuals/keeper/perpetual.go

📚 Learning: 2024-11-15T16:00:11.304Z

Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2551
File: protocol/x/subaccounts/keeper/subaccount.go:852-865
Timestamp: 2024-11-15T16:00:11.304Z
Learning: The function `GetCrossInsuranceFundBalance` in `protocol/x/subaccounts/keeper/subaccount.go` already existed and was just moved in this PR; changes to its error handling may be out of scope.

Applied to files:

protocol/x/perpetuals/keeper/perpetual.go

📚 Learning: 2024-11-15T15:59:28.095Z

Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2551
File: protocol/x/subaccounts/keeper/subaccount.go:833-850
Timestamp: 2024-11-15T15:59:28.095Z
Learning: The function `GetInsuranceFundBalance` in `protocol/x/subaccounts/keeper/subaccount.go` already existed and was just moved in this PR; changes to its error handling may be out of scope.

Applied to files:

protocol/x/perpetuals/keeper/perpetual.go

📚 Learning: 2025-01-25T19:55:32.807Z

Learnt from: UnbornAztecKing
Repo: dydxprotocol/v4-chain PR: 2704
File: protocol/streaming/full_node_streaming_manager.go:205-218
Timestamp: 2025-01-25T19:55:32.807Z
Learning: In the streaming manager's subaccount filtering logic, `slices.Contains` is used instead of a map-based set as it's an optimization for small sets of target subaccountIds, where the overhead of creating and maintaining a hash map would outweigh the benefits.

Applied to files:

protocol/x/perpetuals/keeper/perpetual.go
protocol/x/clob/keeper/grpc_query_leverage.go

📚 Learning: 2025-10-06T20:00:48.549Z

Learnt from: anmolagrawal345
Repo: dydxprotocol/v4-chain PR: 3079
File: protocol/x/clob/types/leverage.go:10-56
Timestamp: 2025-10-06T20:00:48.549Z
Learning: In the leverage update flow for protocol/x/clob, the `ValidateUpdateLeverageMsg` function performs basic validation (non-nil checks, non-zero leverage, unique IDs, valid clob pair existence), while maximum leverage validation against `GetMaxLeverageForPerpetual` is intentionally deferred to the `UpdateLeverage` keeper method.

Applied to files:

protocol/x/clob/keeper/grpc_query_leverage.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Summary

🔇 Additional comments (4)

protocol/x/prices/client/testutil/util.go (1)

4-4: LGTM! Clean refactor to slices.Sort.

The change from sort.Slice to slices.Sort correctly preserves the ascending order sorting behavior while improving code conciseness.

Also applies to: 15-15

protocol/x/clob/keeper/grpc_query_leverage.go (1)

5-5: LGTM! Deterministic ordering preserved.

The refactor to slices.Sort correctly maintains the deterministic ascending order for the leverage response keys while simplifying the code.

Also applies to: 40-40

protocol/x/perpetuals/keeper/perpetual.go (1)

7-8: LGTM! Both imports correctly retained.

The refactor to slices.Sort for the []int32 slice is appropriate. The sort package import is correctly retained for the custom struct sorting at lines 251-253 and 1403-1405.

Also applies to: 619-619

protocol/lib/math.go (1)

8-8: Verification confirmed: Go version requirement is met.

The project's go.mod specifies go 1.23.1, which exceeds the minimum Go 1.21 requirement for slices.Sort. The refactor is safe and appropriate.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ledigang · 2025-11-21T10:25:53Z

refactor label

ledigang · 2025-11-24T07:36:03Z

@tqin7 Hi, Could you please review this PR at your convenience? Thank you very much.

refactor: replace sort.Slice with slices.Sort for natural ordering

4e8c407

Signed-off-by: ledigang <shuangcui@msn.com>

ledigang requested a review from a team as a code owner November 21, 2025 10:25

github-actions bot added the protocol label Nov 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: replace sort.Slice with slices.Sort for natural ordering#3255

refactor: replace sort.Slice with slices.Sort for natural ordering#3255
ledigang wants to merge 1 commit intodydxprotocol:mainfrom
ledigang:main

ledigang commented Nov 21, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 21, 2025 •

edited

Loading

Uh oh!

ledigang commented Nov 21, 2025

Uh oh!

ledigang commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Conversation

ledigang commented Nov 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist

Test Plan

Author/Reviewer Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

ledigang commented Nov 21, 2025

Uh oh!

ledigang commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

ledigang commented Nov 21, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 21, 2025 •

edited

Loading